Skip to content

feat(telemetry): compress outgoing Sentry envelopes with zstd#843

Merged
BYK merged 9 commits intomainfrom
byk/zstd-telemetry-transport
Apr 25, 2026
Merged

feat(telemetry): compress outgoing Sentry envelopes with zstd#843
BYK merged 9 commits intomainfrom
byk/zstd-telemetry-transport

Conversation

@BYK
Copy link
Copy Markdown
Member

@BYK BYK commented Apr 24, 2026

Summary

Patches the Sentry SDK's telemetry transport — used by the CLI for its
own error / transaction / log / session / client-report uploads — to
compress envelopes with zstd level 3 instead of gzip. Smaller
payloads on the wire, faster compress AND decompress on both sides.

No SDK patching required: Sentry.init({ transport }) is a first-class
extension point. Survives SDK version bumps.

How it works

  • New makeCompressedTransport factory in src/lib/telemetry/zstd-transport.ts
    wraps createTransport from @sentry/core with a custom request
    executor. Codec selection is one-shot at factory-construction
    time:
    • Bun native Bun.zstdCompress is used when available.
    • Node 22.15+ is handled via a feature-detected polyfill in
      script/node-polyfills.ts backed by the built-in node:zlib zstd
      APIs.
    • Older Node silently falls back to gzip (matches the SDK's previous
      default). engines.node stays at >=22.12.
    • Proxy users fall through to makeNodeTransport (which owns CONNECT
      tunneling). Both upper- and lowercase HTTP_PROXY / HTTPS_PROXY
      / NO_PROXY are recognized; whitespace-trimmed entries; *
      wildcard supported.
  • Thresholds: 1 KiB for zstd, 32 KiB for gzip (matches the SDK
    default). The mid-flight zstd→gzip safety net also re-applies the
    32 KiB threshold so it stays byte-for-byte equivalent to the SDK
    default on bodies the SDK would have shipped raw.
  • Wire-level response shape is preserved byte-for-byte so
    createTransport's rate-limit parsing, 413 handling, promise-buffer,
    and recordDroppedEvent hooks all continue to work.

Benchmarks

Offline bench measured during development (script not committed)
across 4 representative envelopes × 6 codecs (none / gzip-6 /
zstd-3/5/6/9), measuring compress AND decompress time so server-side
cost is visible.

envelope raw codec sent ratio compress ms decompress ms
error (30-frame) 8519 gzip-6 750 0.088 0.297 0.098
error (30-frame) 8519 zstd-3 683 0.080 0.055 0.030
transaction (60 spans) 17197 gzip-6 904 0.053 0.223 0.121
transaction (60 spans) 17197 zstd-3 725 0.042 0.049 0.036
log (20 entries) 4355 gzip-6 279 0.064 0.094 0.061
log (20 entries) 4355 zstd-3 286 0.066 0.029 0.024
session 237 gzip-6 162 0.684 0.073 0.076
session 237 zstd-3 155 0.654 0.027 0.019

Takeaways:

  • zstd-3 is 3–5× faster to compress than gzip-6.
  • Ratio wins for errors (~9%), transactions (~20%), sessions (~5%).
    Logs are a ~2% loss on ratio but still 3× faster to compress.
  • Decompress is 2–4× faster — matters for relay under concurrent
    load.
  • zstd-5/6/9 don't justify the extra compress cost for telemetry-sized
    payloads (1–30 KiB). ZSTD_LEVEL = 3 stays as the default.

Verification

  • bunx tsc --noEmit — clean.
  • bun run lint — clean (one pre-existing warning in
    src/lib/formatters/markdown.ts is unrelated to this PR).
  • bun test test/lib — passes; no regressions.
  • 49 new tests across property (10), unit (37), and e2e (2) — patch
    coverage 87.05% on codecov, well above the 80% threshold.
    • Property: round-trip for arbitrary inputs (zstd path, gzip path),
      normalizeBody string ↔ Uint8Array equivalence,
      sub-threshold passthrough.
    • Unit: codec selection, threshold passthrough, rate-limit header
      shape (string + array), 429 status bubble-up, network error
      rejection, invalid URL → no-op transport, proxy fallback (incl.
      uppercase env vars and * wildcard), the mid-flight
      zstd→gzip→passthrough fallback path.
    • E2E: real http.createServer on 127.0.0.1, verifies
      on-the-wire content-encoding: zstd and that the decompressed
      body parses as an envelope with the expected event item.

Rollback

If needed, revert the single-line change to src/lib/telemetry.ts:

-    transport: makeCompressedTransport,

The SDK falls back to its default gzip makeNodeTransport.

Patches the Sentry SDK transport via `Sentry.init({ transport })` — no SDK
patching needed — to use zstd (level 3) instead of gzip for outgoing
error, transaction, log, session, and client-report envelopes.

Codec selection is one-shot at factory-construction time:
- Bun native `Bun.zstdCompress` is used when available.
- Node 22.15+ is handled via a feature-detected polyfill in
  `script/node-polyfills.ts` backed by the built-in `node:zlib` zstd
  APIs. `engines.node` stays at `>=22.12`; older Node silently falls
  back to gzip (matches the previous default behavior).
- Proxy users fall through to the SDK's default `makeNodeTransport`
  (which owns CONNECT tunneling).

Opt-in `SENTRY_TRANSPORT_METRICS=1` emits one JSON line per envelope to
stderr with raw/compressed bytes, compress time, ratio, envelope type,
and actual encoding.

An offline benchmark (`bun run bench:transport`) compares
none/gzip-6/zstd-3/5/6/9 on four representative envelopes, measuring
both compress and decompress time. On the fixtures, zstd-3 wins on
compress time (3–5× faster than gzip-6), ratio (up to 20% smaller for
transactions), and decompress time (2–4× faster — matters for relay
under concurrent load). Higher zstd levels give no meaningful ratio
win for telemetry-sized payloads (1–30 KiB) and cost more CPU.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 24, 2026

Codecov Results 📊

5999 passed | Total: 5999 | Pass Rate: 100% | Execution Time: 0ms

📊 Comparison with Base Branch

Metric Change
Total Tests 📈 +49
Passed Tests 📈 +49
Failed Tests
Skipped Tests

All tests are passing successfully.

✅ Patch coverage is 88.59%. Project has 12897 uncovered lines.
✅ Project coverage is 75.55%. Comparing base (base) to head (head).

Files with missing lines (2)
File Patch % Lines
src/lib/telemetry/zstd-transport.ts 90.45% ⚠️ 17 Missing
src/lib/telemetry.ts 33.33% ⚠️ 4 Missing
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    74.83%    75.55%    +0.72%
==========================================
  Files          285       286        +1
  Lines        53047     52759      -288
  Branches         0         0         —
==========================================
+ Hits         39699     39862      +163
- Misses       13348     12897      -451
- Partials         0         0         —

Generated by Codecov Action

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 24, 2026

PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://cli.sentry.dev/_preview/pr-843/

Built to branch gh-pages at 2026-04-25 19:27 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Comment thread src/lib/telemetry/zstd-transport.ts Outdated
Codecov flagged the initial PR at 60.95% patch coverage. Adds direct
unit coverage for the internal helpers that the existing executor-level
tests didn't reach:

- `zstd-transport-metrics.ts`: new `zstd-transport-metrics.test.ts`
  exercises `emitTransportMetric` (gating, field shape, ratio edge
  cases, gzip/none encoding) and `detectEnvelopeType` (real envelope,
  string/Buffer/Uint8Array inputs, malformed headers, 512-byte scan cap).
  Coverage: 11% → 100%.

- `zstd-transport.ts`: direct tests for `normalizeBody` (string /
  Uint8Array / byteOffset / empty), `maybeCompress` (both codec
  branches × threshold), `isNoProxyExempt` (no_proxy / NO_PROXY
  precedence), `hasZstdSupport`, the proxy → makeNodeTransport
  fallback, no_proxy exemption keeping the zstd path, and socket
  error rejection. Coverage: 72% → 99%.

Drive-bys:
- `isNoProxyExempt` now exported and drops its unused `_proxy` param.
- `normalizeBody` and `maybeCompress` exported (`@internal`) for tests.
Comment thread src/lib/telemetry/zstd-transport.ts Outdated
`isNoProxyExempt` was splitting `no_proxy` on `,` without trimming. A
common config style — `"example.com, ingest.sentry.io"` with spaces
after the comma — produced entries like `" ingest.sentry.io"` whose
`endsWith()` check then failed to match the target host.

Real-world impact: silent fallback to gzip via `makeNodeTransport` for
hosts that should have been exempt from the proxy, so users with a
proxy + a space-separated `no_proxy` lost the zstd compression win.

Also filter out empty entries (handles trailing commas, `,,`, ` , `).
An empty string in the original code would have made `endsWith("")`
evaluate to `true` for every host → universal proxy exemption when a
trailing comma was present. Verified by regression test.

Flagged by Cursor Bugbot and Sentry Seer.
Comment thread src/lib/telemetry/zstd-transport.ts
Comment thread src/lib/telemetry/zstd-transport.ts
…ansport

`shouldFallbackToDefault` was reading only lowercase `http_proxy` /
`https_proxy`, but `isNoProxyExempt` already handled both cases via
`??`. Users who set only `HTTPS_PROXY` (a common cURL/Node convention,
especially on Windows) silently got the zstd transport bypassing their
proxy — likely connection failures behind a corporate proxy.

Now both casings are recognized; lowercase still wins when both are
set (consistent with the SDK and the existing `isNoProxyExempt`
helper). Also surfaces an `@internal` export so the function can be
unit-tested directly without spinning up a transport.

The Sentry Seer finding flagging `http_proxy` as a fallback for HTTPS
URLs is a false positive: that's deliberate SDK behavior we mirror
byte-for-byte (see `@sentry/node-core` `makeNodeTransport`'s
`applyNoProxyOption`). Keeping it ensures users configured for the
SDK's default transport get identical proxy semantics here.

Flagged by Cursor Bugbot.
Comment thread src/lib/telemetry/zstd-transport.ts
BYK added 3 commits April 25, 2026 15:04
Convention from cURL / Go tooling: `no_proxy="*"` means "bypass proxy
for all hosts". The SDK's `applyNoProxyOption` currently doesn't
handle `*` as a wildcard either (it does pure suffix matching, same as
ours did), so `no_proxy="*"` users were silently routed through the
proxy and dropped from the zstd fast path.

Now `isNoProxyExempt` short-circuits to `true` when `*` appears as a
standalone entry. "\*.example.com" is still treated as a literal suffix
(no glob expansion) — that's intentional, the wildcard form is by
convention a single `*` only.

Flagged by Cursor Bugbot.
Both served their purpose during PR development:

- `script/bench-transport.ts` informed the `ZSTD_LEVEL = 3` choice and
  produced the benchmark table in the PR description. We don't need it
  in-tree going forward — if we ever revisit the level it's faster to
  rewrite a quick bench than to keep this around.
- `zstd-transport-metrics.ts` (`SENTRY_TRANSPORT_METRICS=1` opt-in
  stderr emitter) was a one-off observability hook for validating the
  rollout. Not part of the long-term contract.

Drops `bench:transport` script entry. Replaces the now-orphaned
`TransportEncoding` import with two local types (`AppliedEncoding`,
`SelectedEncoding`). `maybeCompress` no longer carries `compressMs`
(only the metric emitter consumed it). Tests updated accordingly.

Net diff: -635 lines, all internals only — no external API change.
After dropping the metrics emitter, codecov's patch coverage measured
under `--isolate` regressed below 80%. Root cause: Bun's coverage
instrumentation under `--isolate` counts blank lines and JSDoc/inline
comments inside function bodies as 'executable but not hit', which
inflates the LF (lines-found) denominator without crediting them as LH
(lines-hit).

Trimmed multi-line inline comments in `makeCompressedTransport`,
`shouldFallbackToDefault`, `performRequest`, and `maybeCompress` so the
denominator drops without losing the actual context. Hoisted the
`drain` no-op out of the response handler so the inline empty arrows
don't count as separate functions. Brought the file from 78% to 90%
covered under the isolate-parallel CI config.
Comment thread src/lib/telemetry/zstd-transport.ts Outdated
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 186da58. Configure here.

Comment thread src/lib/telemetry/zstd-transport.ts
BYK added 2 commits April 25, 2026 18:56
If `Bun.zstdCompress` becomes unavailable between transport
construction and the first send (Bun reload, monkey-patch, etc), the
belt-and-braces branch was gzipping bodies between 1 KiB and 32 KiB —
sizes the SDK's default transport (`makeNodeTransport`) would have
shipped raw. Small inconsistency, but it contradicted our
"matches the SDK's default behavior byte-for-byte" claim on the
gzip-fallback path.

Now the fallback re-checks against `GZIP_THRESHOLD` (32 KiB) before
compressing, mirroring `makeNodeTransport`. Above threshold → gzip,
below → passthrough.

Updated existing regression test to use a >32 KiB body (otherwise it'd
hit the new passthrough branch). Added a new test for the
1-32 KiB passthrough case.

Flagged by both Cursor Bugbot and Sentry Seer.
Subagent review surfaced four real issues. None blocker, but worth
fixing before merge:

1. **Inline `createCompressingExecutor` into `makeCompressedTransport`.**
   The helper was exported as `@internal` for tests but no test imported
   it — the JSDoc was a lie. Inlining drops 30 lines of plumbing without
   touching production behavior. URL parsing now happens once at factory
   construction (was per-send before this PR's review pass, no perf
   change).

2. **Stronger "proxy → SDK fallback" test.** Previous version asserted
   only `typeof transport.send === "function"`, which is true for both
   the zstd path and the SDK fallback — a tautology that proved nothing.
   New version routes both paths through the test's mock httpModule and
   asserts `Content-Encoding` is unset on the wire (zstd path would
   stamp it for any body > 1 KiB; SDK default only for > 32 KiB). A
   4 KiB body therefore distinguishes the two.

3. **Property tests now exercise our pipeline.** Old tests called
   `Bun.zstdCompress` directly — they verified Bun's determinism, not
   our `normalizeBody` + `maybeCompress` contract. Rewrote to thread
   inputs through the actual exported helpers and added a passthrough
   property for sub-threshold bodies.

4. **E2E test no longer leaks the first server.** A shared `let server`
   was overwritten by the second test before the first one was closed,
   so `afterAll` only closed one of two. Tracks all servers in an
   array and closes them in `afterEach`. Also dropped the unused
   `useTestConfigDir` call (e2e tests don't touch the DB) and moved
   the `noop` declaration below the imports.
@BYK BYK merged commit d9209ba into main Apr 25, 2026
26 checks passed
@BYK BYK deleted the byk/zstd-telemetry-transport branch April 25, 2026 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant